test: add Vitest unit tests + CI workflow, fix angular-eslint v22 config#34
Merged
Conversation
Adds the project's first unit tests and a working test harness, plus a GitHub Actions workflow to gate pull requests on lint + tests. Test harness (Karma -> Vitest): - Switch the `test` target to @angular/build:unit-test (runner: vitest), which runs headless in Node/jsdom -- no browser needed in CI. - Add a dedicated `app:build:test` build configuration with zone.js + zone.js/testing polyfills; the default config triggered a top-level-await failure in Angular's generated test-bed init. - Add vitest + jsdom; remove the stale karma.conf.js and test.ts (they referenced the old @angular-devkit/build-angular plugin and an outdated zone.js path); clean up tsconfig.spec.json. - Add .browserslistrc (standard Angular matrix). The project had none, so browserslist fell back to `defaults` (Opera Mini/KaiOS), which lack TLA and made the prod build target dead browsers. Tests (32, all passing): - events.service: pub/sub, multi-handler fan-out, error isolation, all unsubscribe branches. - readingdb.service: day-read event, note add/remove, toggleFavorite direction, EMPTY-when-logged-out, login/logout re-pointing, ngOnDestroy. - material.service: TestBed + HTTP testing for load + caching. Lint fix: - Rewrite eslint.config.mjs to the angular-eslint v22 flat-config stack (angular-eslint + typescript-eslint meta-packages). The earlier deps bump to angular-eslint 22 removed `.configs.recommended` from the plugins, which broke `pnpm lint`. CI: - .github/workflows/ci.yml runs on PRs to master: lint/format check first, then unit tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolves the 9 open Dependabot alerts on the default branch. All are transitive deps, so they're pinned through pnpm-workspace.yaml overrides (pnpm 11 ignores package.json overrides). Bumped existing pins: - protobufjs ^7.5.8 -> ^7.6.3 (now 7.6.4) - @grpc/grpc-js ^1.9.15 -> ^1.9.16 (now 1.14.4) - tar ^7.5.11 -> ^7.5.16 (now 7.5.16) Added new pins: - vite ^7.3.5 (dev server / vitest) - esbuild ^0.28.1 (bundler) - @babel/core ^7.29.6 (now 7.29.7) Verified: pnpm lint (0 errors), 32 unit tests pass, prod build, and dev server (HTTP 200) all green with the bumped esbuild/vite toolchain. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
This branch gives the app its first unit tests and a working test harness, plus a GitHub Actions workflow that gates PRs on lint + tests. It also fixes a
pnpm lintregression introduced by a recentangular-eslint21→22 bump.Test harness: Karma → Vitest
The repo had
pnpm testwired to Karma but it couldn't run — no test packages were installed,karma.conf.js/test.tswere stale (referenced the old@angular-devkit/build-angularplugin + outdatedzone.js/dist/zone-testingpath), and there's no browser available. Switched to@angular/build:unit-testwith the Vitest runner, which runs headless in Node/jsdom — no browser needed in CI.app:build:testbuild configuration withzone.js+zone.js/testingpolyfills. The default config triggered a top-level-await failure in Angular's generated test-bed init; the static zone import avoids it. Production build is untouched.vitest+jsdom; removed stalekarma.conf.jsandtest.ts; cleaned uptsconfig.spec.json..browserslistrc(standard Angular matrix). The project had none, so browserslist fell back todefaults(Opera Mini/KaiOS) — those lack TLA and made the prod build needlessly target dead browsers.Tests — 32, all passing
events.service.spec.tsunsubscribebranchesreadingdb.service.spec.tsEVENT_FINISHED_READING, note add/remove,toggleFavoritedirection (union vs remove),userDocValue→EMPTYwhen logged out, login/logout re-pointing,ngOnDestroy, and writes are no-ops when logged outmaterial.service.spec.tsTestBed+ HTTP testing for load + cachingTextSelectDirectivewas intentionally skipped — it relies onSelection/Rangegeometry that jsdom implements poorly.Lint fix (regression)
angular-eslintv22 removed.configs.recommendedfrom the@angular-eslint/*plugins (presets moved to theangular-eslintmeta-package), which brokepnpm lintwithCannot read properties of undefined (reading 'recommended'). Rewroteeslint.config.mjsto the canonical v22 flat-config stack (angular-eslint+typescript-eslintmeta-packages), preserving the project's deliberate overrides (prefer-standalone/prefer-injectoff,no-explicit-anyas warning).CI
.github/workflows/ci.ymlruns on PRs tomaster: checkout → pnpm → Node 22 → install → lint/format check → unit tests (lint gates tests). Concurrency cancels superseded runs.Reviewer notes
src/assets/**(static reading HTML) breaks its parser — aprettier --checkgate would fail CI repo-wide. ESLint is the project's real style/lint gate. Happy to add a scoped Prettier check + format pass if wanted.pnpm lint(0 errors, only trackedno-explicit-anywarnings),pnpm exec ng test --watch=false(32 pass),ionic build --prodall green.Dependabot CVE fixes (added)
Also resolves the 9 open Dependabot alerts (4 high, 3 moderate, 2 low) on the default branch. All are transitive deps, so they're pinned via
pnpm-workspace.yamloverrides (pnpm 11 ignorespackage.jsonoverrides).Bumped existing pins:
^7.5.8^7.6.3(7.6.4)^1.9.15^1.9.16(1.14.4)^7.5.11^7.5.16(7.5.16)New pins:
^7.3.5^0.28.1^7.29.6(7.29.7)The esbuild/vite bumps touch the build toolchain, so verified the full pipeline stays green:
pnpm lint(0 errors), 32 unit tests, prod build, and dev server (HTTP 200).